Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor FossID config #9745

Merged
merged 5 commits into from
Jan 15, 2025
Merged

Refactor FossID config #9745

merged 5 commits into from
Jan 15, 2025

Conversation

mnonnenmacher
Copy link
Member

@mnonnenmacher mnonnenmacher commented Jan 14, 2025

Refactor the FossID config to prepare for migrating scanners to the new plugin API. See the commit messages for details.

@mnonnenmacher mnonnenmacher changed the title refactor(fossid)!: Replace namingProjectPattern with projectName Refactor FossID config Jan 14, 2025
Copy link

codecov bot commented Jan 14, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 68.08%. Comparing base (5d8fb72) to head (7c29ab3).
Report is 15 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##               main    #9745   +/-   ##
=========================================
  Coverage     68.08%   68.08%           
  Complexity     1293     1293           
=========================================
  Files           249      249           
  Lines          8845     8845           
  Branches        923      923           
=========================================
  Hits           6022     6022           
  Misses         2434     2434           
  Partials        389      389           
Flag Coverage Δ
funTest-docker 65.02% <ø> (ø)
funTest-non-docker 33.30% <ø> (ø)
test-ubuntu-24.04 35.91% <ø> (ø)
test-windows-2022 35.89% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mnonnenmacher mnonnenmacher force-pushed the refactor-fossid-config branch from 357b58a to 1c7d785 Compare January 15, 2025 16:51
@mnonnenmacher mnonnenmacher marked this pull request as ready for review January 15, 2025 16:52
@mnonnenmacher mnonnenmacher requested a review from a team as a code owner January 15, 2025 16:52
@mnonnenmacher
Copy link
Member Author

@sschuberth Would be nice to get this into the release tomorrow, then all breaking FossID config changes would happen in a single release.

@sschuberth
Copy link
Member

@sschuberth Would be nice to get this into the release tomorrow, then all breaking FossID config changes would happen in a single release.

Looks like OrtConfigurationTest fails, @mnonnenmacher.

Copy link
Member

@sschuberth sschuberth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only some nits.

model/src/main/resources/reference.yml Show resolved Hide resolved
plugins/scanners/fossid/src/main/kotlin/FossIdConfig.kt Outdated Show resolved Hide resolved
plugins/scanners/fossid/src/main/kotlin/FossIdConfig.kt Outdated Show resolved Hide resolved
Remove the functionality to define a pattern to determine the project
name and instead make the project name configurable as a constant value,
because in practice it turned out that the flexibility provided by the
pattern is not required.

If no `projectName` is configured, it is determined from the Git
repository URL as before.

This change requires adaptations in the "delete newly triggered scans
if a package cannot be scanned" test, because it used mocking to make
the removed `createProjectCode` function behave in a way it would not
behave at runtime.

Signed-off-by: Martin Nonnenmacher <[email protected]>
Add support for `projectName` as a built-in variable for the
`namingScanPattern`. This allows to easily include the project name in
the generated scan code.

Signed-off-by: Martin Nonnenmacher <[email protected]>
Remove support for adding custom naming variables prefixed with
`namingVariable`. Instead, custom values can be written directly into
the `namingScanPattern`.

Signed-off-by: Martin Nonnenmacher <[email protected]>
@mnonnenmacher mnonnenmacher force-pushed the refactor-fossid-config branch from 1c7d785 to 3e6de50 Compare January 15, 2025 19:09
@mnonnenmacher mnonnenmacher enabled auto-merge (rebase) January 15, 2025 19:09
sschuberth
sschuberth previously approved these changes Jan 15, 2025
Replace the custom options to define URL mappings with a single
comma-separated option `urlMappings`. This will simplify the migration
to the new plugin API.

Signed-off-by: Martin Nonnenmacher <[email protected]>
This allows handling the property like all other config properties for
consistency and removes the need to store the plain `options` in the
config class.

Signed-off-by: Martin Nonnenmacher <[email protected]>
@mnonnenmacher mnonnenmacher merged commit 5c67bcd into main Jan 15, 2025
26 checks passed
@mnonnenmacher mnonnenmacher deleted the refactor-fossid-config branch January 15, 2025 20:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants